Skip to content

[PM-40510] feat: rename vault-filter query param to sharedFolderId#21968

Open
jaasen-livefront wants to merge 2 commits into
mainfrom
vault/pm-40510-shared-folder-query-param
Open

[PM-40510] feat: rename vault-filter query param to sharedFolderId#21968
jaasen-livefront wants to merge 2 commits into
mainfrom
vault/pm-40510-shared-folder-query-param

Conversation

@jaasen-livefront

Copy link
Copy Markdown
Collaborator

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-40510

📔 Objective

Behind the vfo1-foundation feature flag (off by default), rename the vault-filter query param from collectionId to sharedFolderId in the shared RoutedVaultFilterService:

  • Reader (filter$) accepts both sharedFolderId and legacy collectionId (preferring the new one), so existing/bookmarked ?collectionId= links keep working during the transition. The RoutedVaultFilterModel.collectionId field name is unchanged — only the URL param is renamed.
  • createRoute() writes sharedFolderId and nulls collectionId when the flag is on; writes collectionId and nulls sharedFolderId when off (queryParamsHandling: "merge" clears the stale param).

Split from PM-40252 (terminology-only) — it is the only routing/behavioral change and is coupled to the per-client reader/writer migrations. App-level direct query-param readers/writers (apps/web vault.component, vault-collection-actions.service; apps/browser new-item-page) are migrated in the per-client tickets; until then the flag stays off, so no broken window.

…erId

Behind the vfo1-foundation flag, RoutedVaultFilterService now reads both
sharedFolderId and legacy collectionId query params (preferring the new one)
and, in createRoute(), writes sharedFolderId while nulling collectionId when
the flag is on (reverse when off). Split from PM-40252 (terminology-only).
@jaasen-livefront jaasen-livefront added the ai-review Request a Claude code review label Jul 20, 2026
@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the rename of the vault-filter query param from collectionId to sharedFolderId in RoutedVaultFilterService, gated behind the vfo1-foundation feature flag (off by default). The reader accepts both params (preferring the new one) for backward compatibility, and createRoute() writes the correct param and nulls the stale one via queryParamsHandling: "merge". Verified the flag defaults to false, the RoutedVaultFilterModel.collectionId field name is preserved so downstream readers are unaffected, and the app-level direct param readers/writers are correctly deferred to per-client tickets. The added spec covers both reader branches, the preference ordering, and both writer flag states.

Code Review Details

No findings. The change is small, symmetric, behind an off-by-default flag, and well tested for the new logic. The type assertions (as CollectionId | null) follow the existing pattern in the file.

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.44%. Comparing base (1c3bb59) to head (295f83a).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
.../vault/src/services/routed-vault-filter.service.ts 60.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #21968      +/-   ##
==========================================
- Coverage   52.44%   52.44%   -0.01%     
==========================================
  Files        4130     4130              
  Lines      130572   130574       +2     
  Branches    20494    20495       +1     
==========================================
  Hits        68475    68475              
+ Misses      57045    57036       -9     
- Partials     5052     5063      +11     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jaasen-livefront jaasen-livefront added the t:feature Change Type - Feature Development label Jul 20, 2026
@jaasen-livefront
jaasen-livefront marked this pull request as ready for review July 22, 2026 19:07
@jaasen-livefront
jaasen-livefront requested a review from a team as a code owner July 22, 2026 19:07

@jengstrom-bw jengstrom-bw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review t:feature Change Type - Feature Development

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants